feat(firecracker): add migratable read-only virtio-fs - #42
Conversation
27fc85a to
b41daa1
Compare
|
Rebased onto The rebase also exposed that the dedicated virtio-fs E2E image was missing the Validation after the rebase:
The full E2E covered the directory root, read-only virtio-fs mount, native writable ext4 mount, network ACL and DNAT, sandboxd recovery, ten consecutive checkpoints, restore, checkpoint-after-restore chaining, storage quota, and Redis SNAT/DNAT traffic. The run completed successfully. |
hky1999
left a comment
hky1999
left a comment
There was a problem hiding this comment.
Approving — verified on this exact head locally, against the now-merged Firecracker side.
Verification basis: two full Firecracker virtio-fs E2E runs on 03af7c6 (default Full mode, and E2E_FIRECRACKER_CHECKPOINT_MODE=incremental) on a nested-KVM host, pairing this head's rebuilt sandboxd/initrd with the migration-capable Firecracker candidate, a guest kernel rebuilt from the fork's PR#6 config (FUSE_FS/VIRTIO_FS built-in, FUSE_DAX off), and virtiofsd 1.14.0. Both green: virtio-fs directory root with private writable layer, read-only mounts, native writable ext4 mounts, network ACL and DNAT, two daemon kill -9 recoveries, ten consecutive checkpoints with restore of the last, checkpoint-after-restore chaining, and storage quota. The incremental run exercised all three tiers with the virtio-fs device attached (daemon logs show type=SoftDirty window generations with pause=0ms, type=Incremental on the post-restore chain, and lineage-loss → type=Full rebase), so the backend dirty-log integration is covered beyond Full snapshots. The merged akernel-dev/firecracker#6 tree (19dccdd95) is byte-identical to the candidate these runs used, so the verification carries over as-is.
Rebase quality checked: pkg/runtime/firecracker/virtiofs.go has zero drift from the previously reviewed 27fc85a; the changes elsewhere are the correct semantic merge of #46's NativeWritableMounts (storage plan, ValidateStartRequest, protocol, agent) coexisting with the virtio-fs export path. A nice side effect: with #32 and #47 now on main, the E2E recovery cases double as regression coverage for both.
Three non-blocking observations, for the record: the staging read-only remount is not MS_REC (nested submounts stay nominally writable in the staging namespace — backstopped by virtiofsd --readonly and the guest-side MS_RDONLY mount); virtiofsd_path must point at the real virtiofsd executable since process identity is verified via /proc//exe + cmdline (a wrapper script would defeat stop/cleanup/liveness checks — worth a line in the ops docs if not already); after a daemon restart adopts a live instance there is no waitVirtioFS-equivalent liveness watcher, so a virtiofsd that dies post-adoption leaves the guest hung on I/O until timeout. None of these gate the merge.
Post-merge deployment notes: the release bundle must carry the rebuilt guest kernel (the previous release kernel has no FUSE_FS/VIRTIO_FS and compat will refuse restore) plus a packaged virtiofsd 1.14+; hosts enabling the flag need ipset for NetworkACL v2 (03af7c6 aligns the E2E image; bare-metal hosts need the same package); and since the virtiofsd digest enters the compat tuple, changing the virtiofsd binary later will intentionally invalidate older checkpoints.
Ready to merge from our side.
|
Release validation update:
Commit |
|
Reviewed the two new commits — both look right, and folding the v1.16.1-akernel.2 promotion into the PR closes the release follow-up cleanly. On 1a28bd6 specifically: good call. The eager directory-to-EROFS conversion was the one path that destroyed the image manager's lazy loading (it reads the complete merged image before the VM starts), so with virtio-fs available it was strictly dominated — deleting it also removes the derived-artifact GC ownership rules and the both-flags precedence special case, and leaves a simpler storage contract. Net -597 lines. One operational note worth a line in the merge description: CI is 13/13 on 1a28bd6 from our side as well; our local virtio-fs E2E (directory root, Full and incremental chains including checkpoint-after-restore) ran against a source-identical build of the promoted tree, so the verification carries over to the new head. Nothing blocking from our side. |
Allow Firecracker sandboxes to use directory-backed OCI or Nydus root filesystems and explicitly read-only directory mounts. Start one private virtiofsd per sandbox over a read-only staging tree while retaining the existing private ext4 writable layer. Extend the guest protocol and agent to mount the shared filesystem as an overlay lower layer and bind selected exports into the sandbox. Track the backend process across normal lifecycle operations and daemon recovery. Integrate virtiofsd state and compatibility metadata with Firecracker checkpoint artifacts. Restore uses a replacement backend and a private shared live-memory clone so committed checkpoint memory remains immutable. Document the configuration and storage contract, and cover directory roots, read-only mounts, ten rolling checkpoints, restore chaining, and network traffic in unit, privileged, and end-to-end tests. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
Keep the dedicated Firecracker virtio-fs image aligned with the common runtime images after NetworkACL v2 made ipset a required E2E dependency. Without it, the privileged suite exits before exercising lifecycle and checkpoint paths. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
Use the promoted Firecracker bundle containing the migration-capable virtio-fs frontend and its tested guest kernel. The checksum pins the exact candidate validated by the sandboxd runtime suite and AKernel standalone tests. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
Remove the eager OCI and Nydus directory-to-EROFS conversion path and its image-manager artifact metadata. Image-backed Firecracker root filesystems now require virtio-fs and consume the mounted directory directly. Keep support for explicitly supplied EROFS files and drives so callers can still use immutable block-backed roots and mounts without conversion. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
Track the virtiofsd waiter independently from transient process identity checks so startup cannot block after a false negative while the daemon is still running. Reuse the wait result across startup and runtime monitoring to keep os/exec ownership single and deterministic. Attach the namespace sandbox leader before scanning its process group, then move already-forked workers into the sandbox cgroup. This keeps both parent and child processes within the sandbox resource boundary under concurrent startup and restore workloads. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
Allow the Firecracker E2E harness to use a read-only host directory as the stress root filesystem and exercise it with up to 32 concurrent guests. Each guest repeatedly verifies large and small files while one sandbox per round is checkpointed and restored. Assert that all virtiofsd processes remain in sandbox cgroups and preserve the stress-sized interface working set so unrelated network pool shrinking cannot interrupt a long storage soak. Document the controls for using a distill-fs Nydus mount as the source directory. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
d741aa1 to
5065b0e
Compare
Remove the obsolete OCI and Nydus EROFS materialization requirement from the top-level limitations. Document that Firecracker accepts either an explicit EROFS image or a directory exported through the supported virtio-fs path. Signed-off-by: Tianyu Zhou <albert.zty@antgroup.com>
hky1999
left a comment
hky1999
left a comment
There was a problem hiding this comment.
Reviewed the two new commits (fbef175, 5065b0e) on top of our earlier review at 5527539653. Both are solid; supporting merge.
fbef175 (virtiofsd process lifecycle): correctly fixes the double-Wait race by funneling all waits through a single-goroutine firecrackerVirtioFSProcess (standard exec.Cmd pattern), kills the full process group on startup timeout, and migrates already-forked workers into the sandbox cgroup before they can escape. The /proc stat parsing with LastIndexByte(')') handles comm-with-parenthesis correctly. Unit tests cover the multi-waiter reuse and the stat parser.
5065b0e (Nydus storage soak): well-designed concurrency test — 32 parallel guests with content-hash verification loops, per-round cgroup containment assertion (assert_virtiofsd_cgroups), and a checkpoint/restore round-trip that validates virtiofs.state presence. The interface cache sizing to match concurrency prevents the idle-trim from interrupting long soaks. Good catch.
Local verification on our side (new head 5065b0e): build clean, all PR-touched package tests pass (firecracker, internal/server, firecracker-agent, config), fmt/vet clean, and the privileged TestPrepareFirecrackerVirtioFSSharedReadOnly passes with the firecracker_integration tag on our KVM host. CI is 13/13. Nothing blocking — please merge.
2 participants
Summary
Allow Firecracker sandboxes to use directory-backed OCI or Nydus root filesystems and explicitly read-only directory mounts. Each sandbox gets a private read-only virtiofsd export while its existing private ext4 image remains the only writable overlay layer.
Extend the guest protocol and agent to mount virtio-fs as the root overlay lower layer and to bind selected exports into the sandbox. Add backend lifecycle validation, sandboxd restart recovery, path traversal protections, and strict read-only staging mounts.
Integrate
virtiofs.state, backend compatibility metadata, and content digests with Firecracker checkpoint/restore. Restore starts a replacement backend and validates the persisted export layout before loading the VM.This PR depends on akernel-dev/firecracker#6.
Validation
make check-fmtmake vetmake testTestPrepareFirecrackerVirtioFSSharedReadOnlyThe E2E suite covered directory and OCI roots, read-only directory mounts, writable overlay behavior, networking and ACLs, sandboxd crash recovery, ten consecutive checkpoints, restore, checkpoint-after-restore chaining, and Redis traffic. OCI image mounts and writable host-directory sharing remain intentionally unsupported.